Skip to content

Expose more loopback middleware for require#834

Merged
raymondfeng merged 1 commit into
masterfrom
feature/expose-loopback-middleware
Nov 19, 2014
Merged

Expose more loopback middleware for require#834
raymondfeng merged 1 commit into
masterfrom
feature/expose-loopback-middleware

Conversation

@raymondfeng

Copy link
Copy Markdown
Member

/to @bajtos

We need a way to reference loopback.favicon and other middleware from middleware.json.

Comment thread server/middleware/error-handler.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is IMHO rather silly. errorHandler is just an alias for require('errorhandler'). In your middleware.json, you can use errorhandler instead of loopback/error-handler.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine to remove error-handler.

@bajtos bajtos self-assigned this Nov 19, 2014
@bajtos

bajtos commented Nov 19, 2014

Copy link
Copy Markdown
Member

IMO this is the last pull request related to middleware that we need to land before we can release a new minor version.

To speed things up, I am proposing the following:

  • removeserver/middleware/error-handler.js
  • keep server/middleware/favicon.js as it is now
  • add serve-favicon to loopback dependencies
  • land this patch and release v2.8.0

@ritch do you have any opinion on this PR?

@raymondfeng

Copy link
Copy Markdown
Member Author

@bajtos, I'll make the changes based on your proposal. It won't conflict with the discussion at strongloop/loopback-boot#70.

@raymondfeng
raymondfeng force-pushed the feature/expose-loopback-middleware branch from 5ea4ada to e56ec57 Compare November 19, 2014 19:11
@raymondfeng

Copy link
Copy Markdown
Member Author

Done.

Comment thread package.json Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason for pinning down the minor version via ~, instead of supporting all minor versions via ^?

@bajtos

bajtos commented Nov 19, 2014

Copy link
Copy Markdown
Member

One more comment, feel free to land the PR if you have convincing reasons why ~ should be used instead of ^.

@bajtos bajtos assigned raymondfeng and unassigned bajtos Nov 19, 2014
@raymondfeng

Copy link
Copy Markdown
Member Author

I'll merge the PR and fix the dependency conventions in a different one.

@raymondfeng
raymondfeng force-pushed the feature/expose-loopback-middleware branch from e56ec57 to a9b4df1 Compare November 19, 2014 19:32
raymondfeng added a commit that referenced this pull request Nov 19, 2014
…eware

Expose more loopback middleware for require
@raymondfeng
raymondfeng merged commit 62637e8 into master Nov 19, 2014
@raymondfeng
raymondfeng deleted the feature/expose-loopback-middleware branch November 19, 2014 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants